Overhaul of merge conflict tests by lukaskubanek · Pull Request #414 · pointfreeco/sqlite-data

lukaskubanek · 2026-03-05T14:15:48Z

Note

The scenarios in this PR rely on MockCloudDatabase populating CKRecord.modificationDate, which landed in #574. Without it the tests exercise different logic than production.

Summary

This PR replaces the existing merge conflict tests in MergeConflictTests.swift with a new test suite that covers a broader set of scenarios across multiple dimensions:

different fields / same fields
client newer / server newer / equal timestamps
conflict on send / conflict on fetch
ordering on send: fetch before retry / retry before fetch

The new tests expose three issues in the merge conflict resolution logic. These are temporarily marked with withKnownIssue, as the main goal of this PR is to document the current behavior, rather than immediately fixing the bugs.

Improvements

A simpler Post table type (standalone, no parent entity required) instead of Reminder, which depends on RemindersList.
Snapshots assert on container.privateCloudDatabase directly, reducing boilerplate compared to the full container, as the shared database isn’t interesting for these tests.
Conflict scenarios use absolute timestamps (e.g. t=30, t=60) for client modification instead of relative increments. This is now aligned with how CKRecords are being treated.
Test steps are now annotated and ordered to match real-world event sequences. This is most notable with the client and server edits, which used to not be ordered chronologically.
Local database state is now checked for convergence with the CloudKit state after each scenario resolves.

Bugs Identified

The test suite revealed the following issues in the implementation:

A client edit is stamped inconsistently after a conflict. On conflict on send (test 2) the field keeps its edit time of t=30 since #574. On conflict on fetch (test 4) it arrives at the server with the record-level time of t=60. Whether the edit time or the merge time is the right one is a design decision, see #583. Test 4 asserts the edit time for now, in line with the existing upstream test.
The client wins same-field conflicts even when the server record has a newer modification timestamp, violating the “last edit wins” strategy as reported in #354. The conflict-on-send cases (tests 7, 9, 16) were fixed by #574. The conflict-on-fetch case (test 14) is fixed by #581.
Converging edits keep the older timestamp. When client and server independently arrive at the same value, including both removing it, the field keeps the server’s older stamp even when the client’s edit is newer, because the engine only restamps fields whose value changed (tests 10 and 17).

Overview

#	Test Name	Client Edits	Server Edits	Expected Result	Actual Result	Correct?	Replaces
1	`differentFieldsChange_conflictOnSend_clientNewer`	`isPublished` @ t=60	`title` @ t=30	Both merged	Both merged	Yes	`serverRecordUpdatedBeforeClientRecord`
2	`differentFieldsChange_conflictOnSend_serverNewer`	`isPublished` @ t=30	`title` @ t=60	Both merged	Both merged	Open (#583)	`clientRecordUpdatedBeforeServerRecord`
3	`differentFieldsChange_conflictOnFetch_clientNewer`	`isPublished` @ t=60	`title` @ t=30	Both merged	Both merged	Yes	`serverAndClientEditDifferentFields`
4	`differentFieldsChange_conflictOnFetch_serverNewer`	`isPublished` @ t=30	`title` @ t=60	Both merged	Both merged	Open (#583)	New
5	`differentNullableFieldsChange_conflictOnFetch_clientNewer`	`priority` @ t=60	`dueDate` @ t=30	Both merged	Both merged	Yes	`mergeWithNullableFields`
6	`sameFieldChange_conflictOnSend_retryBeforeFetch_clientNewer`	`title` @ t=60	`title` @ t=30	Client	Client	Yes	New
7	`sameFieldChange_conflictOnSend_retryBeforeFetch_serverNewer`	`title` @ t=30	`title` @ t=60	Server	Server	Yes (since #574)	New
8	`sameFieldChange_conflictOnSend_fetchBeforeRetry_clientNewer`	`title` @ t=60	`title` @ t=30	Client	Client	Yes	`serverRecordEditedBeforeClientButProcessedAfterClient`
9	`sameFieldChange_conflictOnSend_fetchBeforeRetry_serverNewer`	`title` @ t=30	`title` @ t=60	Server	Server	Yes (since #574)	New (originally proposed in #353)
10	`sameFieldChange_conflictOnSend_equalValues_clientNewer`	`title` @ t=60	Same `title` @ t=30	Same value, `title` @ t=60	Same value, `title` @ t=30	No	New
11	`sameFieldChange_conflictOnSend_equalValues_serverNewer`	`title` @ t=30	Same `title` @ t=60	Same value, `title` @ t=60	Same value, `title` @ t=60	Yes	New
12	`sameFieldChange_conflictOnSend_equalTimestamps`	`title` @ t=60	`title` @ t=60	Client	Client	Yes	New
13	`sameFieldChange_conflictOnFetch_clientNewer`	`title` @ t=60	`title` @ t=30	Client	Client	Yes	`serverRecordEditedAndProcessedBeforeClient`
14	`sameFieldChange_conflictOnFetch_serverNewer`	`title` @ t=30	`title` @ t=60	Server	Client	No	`serverRecordEditedAfterClientButProcessedBeforeClient` (inverts its expectation, the server should win)
15	`sameFieldChangeAndRemoval_conflictOnSend_clientNewer`	Nulls `body` @ t=60	Changes `body` @ t=30	Client	Client	Yes	New
16	`sameFieldChangeAndRemoval_conflictOnSend_serverNewer`	Nulls `body` @ t=30	Changes `body` @ t=60	Server	Server	Yes (since #574)	New
17	`sameFieldRemoval_conflictOnSend_clientNewer`	Nulls `body` @ t=60	Nulls `body` @ t=30	Both nil, `body` @ t=60	Both nil, `body` @ t=30	No	New
18	`sameFieldRemoval_conflictOnSend_serverNewer`	Nulls `body` @ t=30	Nulls `body` @ t=60	Both nil, `body` @ t=60	Both nil, `body` @ t=60	Yes	New

Next Steps

Tests 2 and 4 follow the outcome of #583. Depending on the design decision there, either the implementation changes or the test flips to the merge time.
#581 fixes the remaining conflict-on-fetch case of the second issue and closes #354.
The third issue needs a separate discussion. Fixing it requires the engine to recognize a client edit even when the server already holds the same value, which calls for an ancestor to compare against. Whether the last-known server record is suitable to play that role is one of the open questions for the conflict resolution work.

Introduce Post table type Add differentFieldsChange_conflictOnSend_clientNewer test Add differentFieldsChange_conflictOnSend_serverNewer test Remove replaced tests Add differentFieldsChange_conflictOnFetch_clientNewer test Add differentFieldsChange_conflictOnFetch_serverNewer test Add convergence assertions for local state Remove replaced test Add sameFieldChange_conflictOnSend_retryBeforeFetch_* tests Add sameFieldChange_conflictOnSend_fetchBeforeRetry_* tests Add sameFieldChange_conflictOnFetch_clientNewer test Add sameFieldChange_conflictOnFetch_serverNewer test Remove replaced tests Add sameFieldChangeAndRemoval_* tests Update test for updating nullable fields Combine local database assertions into a single join query Add sameFieldChange_conflictOnSend_equalTimestamps test Add sameFieldRemoval_conflictOnSend_clientNewer test Add sameFieldRemoval_conflictOnSend_serverNewer test Improve synced row query asserts

lukaskubanek · 2026-11-11T13:36:36Z

@mbrandonw Now that #574 landed and the tests exercise the same logic as production, I was able to submit this PR for review. It’s now rebased onto main and the description has been updated to account for the fix from #574. For background, see the discussion in #573.

…known issue

lukaskubanek · 2026-11-12T12:06:02Z

It turns out that #574 fixed the restamp for the conflict-on-send case only. The conflict-on-fetch case is described in #583 together with a general limitation of storing the in-flight record. I’ve updated the description of this PR accordingly.

lukaskubanek mentioned this pull request Mar 25, 2026

Set CKRecord.modificationDate in MockCloudDatabase #411

Closed

This was referenced Jul 28, 2026

Fix permanent sync divergence when userModificationTime stamps tie #506

Closed

Improve MergeConflictTests coverage and naming #353

Closed

lukaskubanek force-pushed the merge-conflict-tests branch from 42a5682 to e7271a1 Compare November 4, 2026 14:17

lukaskubanek mentioned this pull request Nov 4, 2026

Set CKRecord system fields in MockCloudDatabase #573

Closed

lukaskubanek added 22 commits November 11, 2026 11:22

Introduce Post table type

3c295d4

Add differentFieldsChange_conflictOnSend_clientNewer test

3682b2b

Add differentFieldsChange_conflictOnSend_serverNewer test

23080ff

Remove replaced tests

96c3a26

Add differentFieldsChange_conflictOnFetch_clientNewer test

bb48750

Add differentFieldsChange_conflictOnFetch_serverNewer test

9cc1ae4

Add convergence assertions for local state

b39f2d7

Remove replaced test

8d95765

Add sameFieldChange_conflictOnSend_retryBeforeFetch_* tests

2192caf

Add sameFieldChange_conflictOnSend_fetchBeforeRetry_* tests

7d73e9a

Add sameFieldChange_conflictOnFetch_clientNewer test

8afad22

Add sameFieldChange_conflictOnFetch_serverNewer test

d56f15f

Remove replaced tests

7ae899d

Add sameFieldChangeAndRemoval_* tests

949d691

Revisit test for updating nullable fields

5932b21

Combine local database assertions into a single join query

8025158

Add sameFieldChange_conflictOnSend_equalTimestamps test

0182aee

Add sameFieldRemoval_conflictOnSend_clientNewer test

8cbeb01

Add sameFieldRemoval_conflictOnSend_serverNewer test

4dbebc0

Improve synced row query asserts

6976886

Add sameFieldChange_conflictOnSend_equalValues_* tests

ff4ba25

Adjust expectations for fix from pointfreeco#574

5f45336

lukaskubanek force-pushed the merge-conflict-tests branch from 32ade2e to 5f45336 Compare November 11, 2026 10:40

lukaskubanek marked this pull request as ready for review November 11, 2026 13:27

This was referenced Nov 11, 2026

Data divergence between devices after concurrent offline edits #274

Open

Fix conflict-on-fetch scenario #581

Open

Mark restamp in differentFieldsChange_conflictOnFetch_serverNewer as …

e37bd97

…known issue

lukaskubanek mentioned this pull request Nov 12, 2026

Per-field timestamps of client edits survive only in some scenarios #583

Open

3 of 5 tasks

lukaskubanek added a commit to structuredpath/sqlite-data that referenced this pull request Nov 12, 2026

pointfreeco#414

b3f70c3

Navigation Menu

Repository navigation

Overhaul of merge conflict tests - #414

Overhaul of merge conflict tests#414
lukaskubanek wants to merge 23 commits into
pointfreeco:mainfrom
structuredpath:merge-conflict-tests

lukaskubanek commented Mar 5, 2026 •

edited

Loading

Uh oh!

lukaskubanek commented Nov 11, 2026

Uh oh!

lukaskubanek commented Nov 12, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lukaskubanek commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Improvements

Bugs Identified

Overview

Next Steps

Uh oh!

lukaskubanek commented Nov 11, 2026

Uh oh!

lukaskubanek commented Nov 12, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lukaskubanek commented Mar 5, 2026 •

edited

Loading